-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add library for reset interface #2629
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
Cleaner way of separating the usb_reset_interface.h header
src/common/pico_usb_reset_interface_headers/include/pico/usb_reset_interface.h
Outdated
Show resolved
Hide resolved
#include "stdint.h" | ||
#include "device/usbd_pvt.h" | ||
|
||
void resetd_init(void); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure resetd
is a sufficient/good prefix for the public API
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note also, on this and the below, this is peculiar to TinyUSB - as we think about CherryUSB etc, we (i haven't looked in detail) should consider what if any of this is exposed (and whether we should expose them again in a separate header - e.g. one for TinyUSB one for CherryUSB)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will rename them to pico_usb_reset_interface_...
Note that these functions are not supposed to be exposed as a public API - the only thing that should be used publicly is the _resetd_driver
struct, I will try to refactor it so that's the case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a separate TinyUSB header, and made it so this should be compatible with the CherryUSB PR (as that only uses the usb_reset_interface.h
header)
I couldn't refactor the functions out of the header, because I think the pico_usb_reset_interface_driver
has to be defined in the header as static const
in order for the debugprobe inclusion to compile, and therefore the functions must be in the header too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will rename them to
pico_usb_reset_interface_...
Note that these functions are not supposed to be exposed as a public API - the only thing that should be used publicly is the
_resetd_driver
struct, I will try to refactor it so that's the case
note pico_
/hardware_
is not included in function names; e.g. dma_foo
#endif | ||
#endif | ||
|
||
// PICO_CONFIG: PICO_STDIO_USB_RESET_INTERFACE_MS_OS_20_DESCRIPTOR_ITF, If vendor reset interface is included the USB interface number for the reset interface, type=int, default=2 if application is not using TinyUSB directly, undefined otherwise, group=pico_usb_reset_interface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// PICO_CONFIG: PICO_STDIO_USB_RESET_INTERFACE_MS_OS_20_DESCRIPTOR_ITF, If vendor reset interface is included the USB interface number for the reset interface, type=int, default=2 if application is not using TinyUSB directly, undefined otherwise, group=pico_usb_reset_interface | |
// PICO_CONFIG: PICO_STDIO_USB_RESET_INTERFACE_MS_OS_20_DESCRIPTOR_ITF, If vendor reset interface is included this specifies the USB interface number for the reset interface, type=int, default=2 if application is not using TinyUSB directly, undefined otherwise, group=pico_usb_reset_interface |
Perhaps this is slightly more readable?
// PICO_CONFIG: PICO_STDIO_USB_RESET_INCLUDE_APP_DRIVER_CB, Set to 0 if your application defines usbd_app_driver_get_cb, type=bool, default=1 when using pico_usb_reset_interface, 0 otherwise, group=pico_usb_reset_interface | ||
#ifndef PICO_STDIO_USB_RESET_INCLUDE_APP_DRIVER_CB | ||
#define PICO_STDIO_USB_RESET_INCLUDE_APP_DRIVER_CB LIB_PICO_STDIO_USB | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// PICO_CONFIG: PICO_STDIO_USB_RESET_INCLUDE_APP_DRIVER_CB, Set to 0 if your application defines usbd_app_driver_get_cb, type=bool, default=1 when using pico_usb_reset_interface, 0 otherwise, group=pico_usb_reset_interface | |
#ifndef PICO_STDIO_USB_RESET_INCLUDE_APP_DRIVER_CB | |
#define PICO_STDIO_USB_RESET_INCLUDE_APP_DRIVER_CB LIB_PICO_STDIO_USB | |
#endif | |
// PICO_CONFIG: PICO_STDIO_USB_RESET_INCLUDE_DEFAULT_APP_DRIVER_CB, Set to 0 if your application defines its own usbd_app_driver_get_cb function, type=bool, default=1 when using pico_usb_reset_interface, 0 otherwise, group=pico_usb_reset_interface | |
#ifndef PICO_STDIO_USB_RESET_INCLUDE_DEFAULT_APP_DRIVER_CB | |
#define PICO_STDIO_USB_RESET_INCLUDE_DEFAULT_APP_DRIVER_CB LIB_PICO_STDIO_USB | |
#endif |
Maybe? 🤷♂️
#define _PICO_USB_RESET_INTERFACE_DEVICE_H | ||
|
||
/** \file usb_reset_interface_device.h | ||
* \defgroup pico_usb_reset_interface pico_usb_reset_interface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this new pico_usb_reset_interface
group need adding to docs/index.h
?
This separates the stdio_usb reset interface out into a separate library, which can be used by applications which use TinyUSB directly.
No change in behaviour when using the typical stdio_usb (eg in the hello_usb example).
Keeps the
PICO_STDIO_USB_
prefix on the config defines to maintain compatibility, and also maintains compatibility with picotool.Instructions to use this are added to the header file.
See implementation in debugprobe and the dev_multi_cdc example